feat(skills): configurable SSOT storage location (cc-switch | unified), align with upstream GUI by ShatterDusk · Pull Request #414 · SaladDay/cc-switch-cli

ShatterDusk · 2026-08-15T13:21:58Z

Summary

Add configurable skills SSOT storage location (skill_storage_location: cc-switch | unified) to the CLI, aligning with the upstream GUI's skillStorageLocation feature (farion1231/cc-switch). In unified mode, ~/.agents/skills/ becomes the SSOT — the cross-tool standard directory adopted by skills.sh, LangChain, and six major harnesses.

Closes #413

Motivation

~/.agents/skills/ is the de-facto cross-harness standard for agent skills (Codex, Gemini CLI, DSH, OpenCode, Windsurf, Warp scan it natively; LangChain deepagents and skills.sh adopt it). The CLI's SSOT was hardcoded to {config_dir}/skills, forcing users of the .agents convention into two divergent skill copies. See #413 for the full harness adoption matrix.

Changes

File	Change
`src/services/skill.rs`	`SkillStorageLocation` enum; `get_ssot_dir()` honors location; `migrate_storage()` with pre-migration backup; `validate_skill_storage_destination()`
`src/settings.rs`	`skill_storage_location` field (`#[serde(default)]`), get/set functions
`src/lib.rs`	re-exports for tests/callers
`src/cli/commands/skills.rs`	`skills storage-location` command (get/set + triggers migration)
`tests/skills_service.rs`	tests for default, roundtrip, storage-location CLI

CLI usage

cc-switch skills storage-location              # show current (cc-switch | unified)
cc-switch skills storage-location unified      # switch to ~/.agents/skills + migrate
cc-switch skills storage-location cc-switch    # switch back + migrate

Behavior (aligned with upstream GUI's migrate_storage)

Pre-migration backup of the old SSOT to {config_dir}/skill-backups/ (new: upstream has no backup; added for rollback safety)
Validate destination (rejects aliased app skills dirs via canonicalize)
Move skills one-by-one (rename → copy+delete fallback; soft-fail per skill)
Update setting only after files moved
Re-sync app skill dirs to the new SSOT

Testing

New: 5 unit tests (migrate_storage_*, get_ssot_dir_switches_on_location) + 3 integration tests (storage_location_*) — all pass
services::skill module: 20/20 pass
Full lib: 4088 passed; 16 failed — all 16 failures are pre-existing cli::tui::theme::tests (terminal color detection fails in non-TTY env), unrelated to this change (verified on clean baseline via git stash)
cargo fmt --check clean; cargo clippy no new warnings

Notes

Values: clap kebab-case cc-switch/unified for CLI args; serde snake_case cc_switch/unified in settings.json
Migration backup uses directory copy (not zip) for simplicity — sufficient for rollback
migrate_storage traverses the SSOT directory (not the index), so traversal-safe by construction

…creation, TUI/CLI error surfacing, clap alias, no-op hint P1-1: copy_dir_recursive failure now cleans partial dst; source cleanup failures logged P1-2: TUI storage-location toast shows errors/skipped count (Warning on failure) P1-3: extract ssot_dir_for(); migrate_storage uses create_managed_config_dir_all P2-1: clap value alias cc_switch for CcSwitch P2-3: CLI no-op early return; error path prints failures + backup hint test: migrate_storage_keeps_source_and_setting_on_failure

SaladDay · 2026-08-18T06:50:01Z

Thanks for the PR! 🙏 It is not ready to merge yet.

Main blockers:

Migration syncs every moved skill to all apps, ignoring the configured app selection.
It moves unmanaged folders from ~/.agents/skills, while upstream only migrates database-managed skills.
If the destination already contains the skill, app symlinks can remain pointed at the old storage.
The TUI should show a confirmation before migrating existing skills.
The full test suite has additional compilation errors caused by missing test stubs.

Formatting and the new targeted tests pass, but these migration issues should be fixed first. I recommend following the upstream backend behavior and keeping the TUI consistent with the existing confirmation/help patterns. 🙂

…app sync, symlink refresh, TUI confirm dialog, text clarity - migrate_storage now iterates DB-managed skills only (not unmanaged dirs) - post-migration sync respects each skill's enabled app matrix - destination-exists case still refreshes app symlinks to new SSOT - add require_valid_directory traversal guard - TUI: confirm dialog before migration (ConfirmAction::SkillsMigrateStorage) - TUI: rename label to 技能存储位置 / Skill storage location - test: migrate_storage_skips_unmanaged_dirs

ShatterDusk · 2026-08-20T23:37:36Z

Thanks for the detailed review — all four migration blockers are addressed in the latest commits (cbee62fd).

1. Migration now respects the configured app selection
Post-migration sync reads each skill's enabled app matrix from the DB (record.apps.is_enabled_for(app)) instead of syncing to all apps.

2. Only DB-managed skills are migrated
migrate_storage now iterates load_index().skills (the managed set) instead of fs::read_dir, so unmanaged folders under the SSOT are left untouched. Added a regression test (migrate_storage_skips_unmanaged_dirs). Also added require_valid_directory to guard against traversal-polluted DB rows.

3. Destination-exists case refreshes app symlinks
When the destination already contains a skill, the files are skipped (as before) but the skill is still queued for app re-sync, so symlinks are repointed at the new SSOT instead of lingering on the old one.

4. TUI confirmation before migration
The storage-location picker now opens the existing Overlay::Confirm dialog (with the standard ConfirmAction dispatch) before issuing the migration action. The settings label was also clarified to "技能存储位置 / Skill storage location" so it's clear what is being relocated.

5. Full test suite compilation errors
These appear to be pre-existing on the base branch: git worktree at origin/main (before this PR's commits) fails the same way on crate::test_support (lib.rs declares it as pub(crate) under #[cfg(test)], but tests/openclaw_config.rs:252 defines a local mod test_support that shadows it for that integration target) and on AppError::localized. cargo test --lib compiles and passes for our changes (22/22 in services::skill, plus the new TUI picker tests). I did not touch those pre-existing failures to keep this PR scoped — happy to fix them separately if you'd like.

Formatting and targeted tests pass; cargo clippy adds no new warnings.

ShatterDusk · 2026-08-21T00:01:11Z

Added soap-opera tests for migrate_storage (commit ad19ec27) — four disaster scenes stacking extreme conditions: managed/unmanaged/hidden/traversal-DB-row + destination conflict + symlink repointing in one migration; pre-occupied backup dir; empty-DB migration; and a roundtrip that must not drag unmanaged dirs back. All pass (26/26 in services::skill). These are the scenarios your blockers pointed at, now locked in as regressions.

ShatterDusk · 2026-08-21T00:28:24Z

Independent audit (mutation testing) of the test suite, then fixed what it found (commit c44d2dc3):

Core migration tests verified real — mutating migrate_storage to no-op / no-file-move / no-backup / no-symlink-refresh each turned the expected tests red.
One empty assertion found & fixed: the soap-opera traversal check asserted on a path that never existed (always true regardless of the guard). Now pre-creates the traversal target and asserts it survives + error is recorded; removing the guard turns the test red (verified).
Added a pure unit test for require_valid_directory (traversal/absolute/hidden/empty cases).
Integration tests storage_location_* only guard settings persistence (not migration) — noted in code comments to avoid false coverage claims.

27/27 tests pass in services::skill.

ShatterDusk added 7 commits August 15, 2026 20:43

feat(settings): add skill_storage_location enum, field, get/set

9cb3ef2

feat(skill): get_ssot_dir honors skill_storage_location

a3c4d20

style: cargo fmt for skills_service tests

a662b6d

feat(skill): add migrate_storage with pre-migration backup

3c792a2

feat(cli): add skills storage-location command

d43e5d1

feat(tui): add skills storage-location picker and settings entry

2af7a5d

BrewTestBot mentioned this pull request Aug 18, 2026

cc-switch-cli 5.10.2 Homebrew/homebrew-core#299469

Merged

Navigation Menu

feat(skills): configurable SSOT storage location (cc-switch | unified), align with upstream GUI - #414

feat(skills): configurable SSOT storage location (cc-switch | unified), align with upstream GUI#414
ShatterDusk wants to merge 8 commits into
SaladDay:mainfrom
ShatterDusk:feat/skills-ssot-location

ShatterDusk commented Aug 15, 2026

Uh oh!

SaladDay commented Aug 18, 2026 •

edited

Loading

Uh oh!

ShatterDusk commented Aug 20, 2026

Uh oh!

ShatterDusk commented Aug 21, 2026

Uh oh!

ShatterDusk commented Aug 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

ShatterDusk commented Aug 15, 2026

Summary

Motivation

Changes

CLI usage

Behavior (aligned with upstream GUI's migrate_storage)

Testing

Notes

Uh oh!

SaladDay commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ShatterDusk commented Aug 20, 2026

Uh oh!

ShatterDusk commented Aug 21, 2026

Uh oh!

ShatterDusk commented Aug 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

SaladDay commented Aug 18, 2026 •

edited

Loading